Skip to content

feat: add mailman stop command for graceful daemon shutdown#34

Merged
UserAd merged 1 commit intomainfrom
012-mailman-stop
Jan 20, 2026
Merged

feat: add mailman stop command for graceful daemon shutdown#34
UserAd merged 1 commit intomainfrom
012-mailman-stop

Conversation

@UserAd
Copy link
Owner

@UserAd UserAd commented Jan 20, 2026

Summary

  • Add agentmail mailman stop subcommand to gracefully stop the mailman daemon
  • Implement file-based signaling using .agentmail/.stop file
  • Daemon detects stop file via existing fsnotify watcher and initiates shutdown
  • Uses atomic file creation (O_CREATE|O_EXCL) to handle "stop already pending" case

Changes

New Files

  • internal/cli/mailman_stop.go - Stop command implementation
  • internal/cli/mailman_stop_test.go - Tests for stop command (4 tests)
  • specs/012-mailman-stop/ - Feature specification and design documents

Modified Files

  • internal/daemon/daemon.go - Add StopFile constant, StopFilePath(), file-based shutdown handling
  • internal/daemon/watcher.go - Add StopChan() method, stop file detection in Run()
  • internal/daemon/watcher_test.go - Add tests for stop file detection (2 tests)
  • cmd/agentmail/main.go - Register stop subcommand under mailman

Test plan

  • All existing tests pass (go test -v -race ./...)
  • New tests for MailmanStop command pass
  • New tests for stop file detection pass
  • Quality gates pass: gofmt, go vet, govulncheck, gosec
  • Manual test: agentmail mailman stop creates .stop file and outputs "Stop signal sent"
  • Manual test: Running stop when file exists outputs "Stop already pending" with exit code 1

Implement file-based signaling mechanism to stop the mailman daemon:
- Add `agentmail mailman stop` subcommand that creates .agentmail/.stop file
- Daemon's file watcher detects .stop file and initiates graceful shutdown
- Daemon removes both .stop and .pid files during shutdown
- Uses atomic file creation (O_CREATE|O_EXCL) to detect "stop already pending"

Exit codes: 0 (success), 1 (error/already pending)
@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

This PR implements a file-based stop mechanism for the Mailman daemon. It introduces a new mailman stop CLI subcommand that creates a .stop signal file in the .agentmail directory. The daemon's file watcher detects this file creation and initiates graceful shutdown. Changes include CLI handler, watcher enhancements, daemon shutdown coordination, tests, and comprehensive specifications.

Changes

Cohort / File(s) Summary
CLI Command
cmd/agentmail/main.go, internal/cli/mailman_stop.go, internal/cli/mailman_stop_test.go
New stopCmd subcommand registered under mailman; MailmanStop() handler atomically creates .stop file using os.OpenFile(O_CREATE|O_EXCL) with error handling for pending stops and filesystem errors; exit code 0 on success, 1 on failure; includes 4 unit tests covering success path, already-pending, and error cases.
Daemon Stop Signal
internal/daemon/daemon.go, internal/daemon/watcher.go, internal/daemon/watcher_test.go
New StopFile constant and StopFilePath() helper added to daemon; FileWatcher extended with fileStopChan channel and StopChan() method to expose shutdown signal; isStopFileEvent() detects .stop file creation via fsnotify; runForeground() now selects on stop signal and performs cleanup (removes stop and PID files); includes 2 new watcher tests for stop file detection.
Documentation & Specifications
CLAUDE.md, specs/012-mailman-stop/*
Updated CLAUDE.md with 012-mailman-stop entry (Go 1.25.5, standard library only); added 9 new spec documents covering requirements checklist, CLI contract, data-model, implementation plan, quickstart, research, feature spec, and task breakdown; documents file-based IPC design, error scenarios, and transition from SIGTERM approach.

Sequence Diagram

sequenceDiagram
    actor User
    participant CLI as CLI<br/>(mailman stop)
    participant FS as File System<br/>(.agentmail/.stop)
    participant Watcher as FileWatcher<br/>(fsnotify)
    participant Daemon as Daemon<br/>(runForeground)

    User->>CLI: agentmail mailman stop
    CLI->>FS: Create .stop file atomically<br/>(O_CREATE|O_EXCL)
    FS-->>CLI: File created (exit 0)
    CLI-->>User: "Stop signal sent"
    
    Watcher->>FS: Monitor .agentmail/
    FS-->>Watcher: fsnotify.Create event<br/>for .stop
    Watcher->>Watcher: Detect stop file event
    Watcher-->>Daemon: Close fileStopChan
    
    Daemon->>Daemon: Select on StopChan()<br/>receives close
    Daemon->>FS: Remove .stop file
    Daemon->>FS: Remove PID file
    Daemon->>Daemon: Graceful shutdown
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a mailman stop command for graceful daemon shutdown. It is concise, specific, and clearly conveys the primary feature introduced in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@internal/cli/mailman_stop_test.go`:
- Around line 158-162: Replace the manual length check and slicing that compares
stderr.String() to expectedPrefix with the idiomatic strings.HasPrefix call:
import the strings package, then use if !strings.HasPrefix(stderr.String(),
expectedPrefix) { t.Errorf(... ) } referencing the existing stderr variable and
expectedPrefix constant in the test (mailman_stop_test.go) to make the intent
clearer and avoid manual bounds checks.

In `@internal/cli/mailman_stop.go`:
- Around line 29-31: The code silently falls back to os.Getwd when FindGitRoot
fails and leaves repoRoot empty if Getwd also fails; update mailman_stop.go to
either log that fallback and any Getwd error (in the block where err != nil and
repoRoot is assigned) or add a clear function comment above the relevant
function noting that repoRoot may be empty if both FindGitRoot and os.Getwd
fail, referencing FindGitRoot, os.Getwd, and repoRoot so future maintainers
understand the edge case.

In `@specs/012-mailman-stop/data-model.md`:
- Around line 21-27: The MD058 lint failure is caused by the table not being
surrounded by blank lines; edit the section containing the "**Operations**:"
heading and the subsequent markdown table so there is an empty line before the
table (between the "**Operations**:" line and the table) and an empty line after
the table, ensuring the table is separated by blank lines for compliant
rendering.

In `@specs/012-mailman-stop/research.md`:
- Around line 89-105: Add a blank line immediately before and immediately after
the fenced code block that shows the "Existing Code Reference
(`internal/daemon/daemon.go:249-271`)" in specs/012-mailman-stop/research.md so
it complies with markdownlint MD031; locate the triple-backtick fence that wraps
the Go snippet (the block containing "<-sigChan" through "return 0") and insert
one empty line above the opening ``` and one empty line below the closing ``` to
fix the lint error.
- Around line 41-53: The fenced Go code block around the atomic create snippet
needs surrounding blank lines to satisfy markdownlint MD031: insert one blank
line immediately before the ```go fence that starts the block and one blank line
immediately after the closing ``` fence so the snippet with
os.OpenFile(stopFilePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) and its
defer/Close/return lines is separated from surrounding text.

In `@specs/012-mailman-stop/tasks.md`:
- Around line 106-111: The fenced code block showing the phase dependency list
lacks a language tag and triggers markdownlint MD040; update the block opener
from ``` to include a language (e.g., use ```text or a more specific language)
so the block containing "Phase 1 (Setup) → No dependencies" through "Phase 4
(Polish) → Depends on all user stories" has a proper language identifier.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c1eb33 and 50752cf.

📒 Files selected for processing (15)
  • CLAUDE.md
  • cmd/agentmail/main.go
  • internal/cli/mailman_stop.go
  • internal/cli/mailman_stop_test.go
  • internal/daemon/daemon.go
  • internal/daemon/watcher.go
  • internal/daemon/watcher_test.go
  • specs/012-mailman-stop/checklists/requirements.md
  • specs/012-mailman-stop/contracts/cli.md
  • specs/012-mailman-stop/data-model.md
  • specs/012-mailman-stop/plan.md
  • specs/012-mailman-stop/quickstart.md
  • specs/012-mailman-stop/research.md
  • specs/012-mailman-stop/spec.md
  • specs/012-mailman-stop/tasks.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.go: Go code must use Go 1.21 or higher (project uses Go 1.25.5)
Go code must use Standard library only - no external dependencies except where explicitly justified (e.g., fsnotify for 009-watch-files, modelcontextprotocol/go-sdk for 010-mcp-server)
All Go code must pass gofmt -l . with no output
All Go code must pass go vet ./... static analysis
All Go code must pass go test -v -race -coverprofile=coverage.out ./... with >= 80% code coverage
All Go code must pass govulncheck ./... with no vulnerabilities reported
All Go code must pass gosec ./... security scanning with no issues reported
Messages must be stored in .agentmail/mailboxes/<recipient>.jsonl JSONL format

Files:

  • internal/daemon/daemon.go
  • internal/daemon/watcher_test.go
  • internal/cli/mailman_stop.go
  • internal/daemon/watcher.go
  • internal/cli/mailman_stop_test.go
  • cmd/agentmail/main.go
🧠 Learnings (3)
📚 Learning: 2026-01-18T04:45:41.353Z
Learnt from: CR
Repo: UserAd/AgentMail PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-18T04:45:41.353Z
Learning: Applies to **/*.go : Messages must be stored in `.agentmail/mailboxes/<recipient>.jsonl` JSONL format

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-01-18T04:45:41.353Z
Learnt from: CR
Repo: UserAd/AgentMail PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-18T04:45:41.353Z
Learning: Message files use JSONL format (JSON Lines) with one message per line in `.agentmail/mailboxes/<recipient>.jsonl` with file locking for atomic operations

Applied to files:

  • CLAUDE.md
📚 Learning: 2026-01-18T04:45:41.353Z
Learnt from: CR
Repo: UserAd/AgentMail PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-18T04:45:41.353Z
Learning: Applies to **/*.go : Go code must use Go 1.21 or higher (project uses Go 1.25.5)

Applied to files:

  • CLAUDE.md
🧬 Code graph analysis (6)
internal/daemon/daemon.go (1)
internal/mail/mailbox.go (1)
  • RootDir (15-15)
internal/daemon/watcher_test.go (2)
internal/daemon/watcher.go (1)
  • NewFileWatcher (119-142)
internal/daemon/daemon.go (1)
  • StopFilePath (52-54)
internal/cli/mailman_stop.go (2)
internal/mail/ignore.go (1)
  • FindGitRoot (13-28)
internal/daemon/daemon.go (1)
  • StopFilePath (52-54)
internal/daemon/watcher.go (1)
internal/daemon/daemon.go (1)
  • StopFile (22-22)
internal/cli/mailman_stop_test.go (2)
internal/cli/mailman_stop.go (2)
  • MailmanStop (23-50)
  • MailmanStopOptions (13-15)
internal/daemon/daemon.go (1)
  • StopFilePath (52-54)
cmd/agentmail/main.go (1)
internal/cli/mailman_stop.go (2)
  • MailmanStop (23-50)
  • MailmanStopOptions (13-15)
🪛 LanguageTool
specs/012-mailman-stop/plan.md

[style] ~16-~16: This sentence may be long and difficult for your reader to follow. Consider inserting a period and starting a new sentence here.
Context: ...orm**: macOS and Linux (per constitution, with tmux installed) Project Type: Single CLI tool **Pe...

(WITH_THE_SENTENCE)

specs/012-mailman-stop/tasks.md

[style] ~91-~91: Consider using a different verb for a more formal wording.
Context: ...fmt -w .- [x] T022 [P] Run go vet and fix any issues:go vet ./...` - [x] T023 R...

(FIX_RESOLVE)

specs/012-mailman-stop/research.md

[grammar] ~65-~65: Ensure spelling is correct
Context: ...needed - Detection is nearly instant (< 100ms typically) Implementation Notes: -...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)
specs/012-mailman-stop/data-model.md

22-22: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

specs/012-mailman-stop/tasks.md

106-106: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

specs/012-mailman-stop/research.md

42-42: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


90-90: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🔇 Additional comments (22)
CLAUDE.md (1)

105-106: LGTM!

The documentation accurately reflects the 012-mailman-stop feature dependencies and file paths. The stdlib-only approach aligns with the project's constitution (IC-001).

specs/012-mailman-stop/spec.md (1)

1-85: LGTM!

The specification is comprehensive and well-structured. The requirements (FR-001 through FR-008) align with the implementation, the acceptance scenarios are testable, and the clarifications document important design decisions (fire-and-forget, unconditional file creation, fsnotify detection).

internal/daemon/daemon.go (3)

21-23: LGTM!

The StopFile constant follows the same pattern as PIDFile and uses a dot-prefix for the hidden file, which is appropriate for a signal file.


51-54: LGTM!

The StopFilePath helper follows the established pattern from PIDFilePath, maintaining consistency across the daemon package.


257-284: LGTM!

The stop signal handling correctly implements FR-005 through FR-008:

  • Monitors fileWatcher.StopChan() for file-based stop signals in both test and production modes
  • Removes the stop file on shutdown (FR-007)
  • Removes the PID file on shutdown (FR-008)

The best-effort cleanup pattern with ignored errors is appropriate here since the daemon is shutting down.

internal/daemon/watcher_test.go (2)

424-467: LGTM!

The test correctly validates the file-based stop detection mechanism:

  • Uses StopFilePath for consistent path resolution
  • 1-second timeout aligns with SC-002 success criteria ("Daemon detects stop file within 1 second")
  • Proper cleanup with defer os.RemoveAll(tmpDir) and fallback fw.Close() in timeout case

469-487: LGTM!

Simple but necessary contract test verifying StopChan() returns a valid channel for external shutdown coordination.

specs/012-mailman-stop/plan.md (1)

1-87: LGTM!

The implementation plan is well-structured with:

  • Clear technical context and constitution compliance check
  • Accurate project structure mapping to actual implementation files
  • Good justification for choosing file-based IPC over SIGTERM (fewer files, fewer dependencies, fewer error scenarios)
cmd/agentmail/main.go (2)

181-206: LGTM!

The stopCmd subcommand is well-implemented:

  • Follows the established pattern for other commands in this file
  • LongHelp clearly documents the file-based signaling mechanism and exit codes
  • Exec properly delegates to cli.MailmanStop with consistent exit code handling

208-243: LGTM!

The mailmanCmd updates cleanly integrate the stop subcommand:

  • ShortUsage clearly shows both start and stop usage patterns
  • LongHelp properly documents the new stop command with examples
  • Exit codes are accurately documented (exit 2 is specific to start when daemon already running)
  • Subcommands array correctly includes stopCmd
specs/012-mailman-stop/checklists/requirements.md (1)

1-66: LGTM! Well-structured specification checklist.

The EARS pattern mapping is clear and the documentation of the transition from SIGTERM to file-based approach provides useful historical context. The reduction from 12 to 8 requirements reflects a simpler, more cross-platform design.

specs/012-mailman-stop/data-model.md (1)

1-127: LGTM! Comprehensive data model documentation.

The state transition diagrams clearly illustrate both the stop command flow and daemon shutdown flow. The comparison table between old (SIGTERM) and new (file-based) approaches effectively communicates the design rationale.

internal/cli/mailman_stop_test.go (3)

16-47: LGTM! Well-structured test for stop file creation.

The test correctly validates that the stop file is created at the expected path using daemon.StopFilePath, ensuring consistency with the daemon's expectations.


91-133: LGTM! Good coverage of the "stop already pending" scenario.

Pre-creating the stop file to simulate the pending state is an appropriate test setup, and the assertions correctly verify both the exit code and error message routing to stderr.


135-168: LGTM! Filesystem error handling is properly tested.

Omitting the .agentmail directory creation is a clean way to trigger the filesystem error path without requiring special permissions or mocking.

internal/cli/mailman_stop.go (1)

23-50: LGTM! Clean implementation with proper atomic semantics.

The use of O_CREATE|O_EXCL correctly ensures that only one stop signal can be pending at a time. The fallback chain (RepoRoot → FindGitRoot → Getwd) provides reasonable defaults while the error handling distinguishes between "file exists" and other filesystem errors.

specs/012-mailman-stop/contracts/cli.md (2)

152-166: Good documentation of edge case behavior.

Explicitly documenting that the stop command succeeds even when no daemon is running, and noting that the .stop file will persist, is important for operational clarity. This fire-and-forget semantic is intentional and well-justified.


1-166: LGTM! Comprehensive CLI contract specification.

The contract clearly defines the command behavior, exit codes, and output formats. The fire-and-forget pattern is explicitly documented with clear separation of responsibilities between the CLI and daemon.

internal/daemon/watcher.go (4)

103-107: LGTM! Clean channel addition for file-based stop signaling.

The fileStopChan is properly initialized as unbuffered and provides a separate signal path from the existing stopChan, maintaining clear separation between external close requests and file-based stop detection.


149-161: LGTM! Well-designed stop file detection.

The StopChan() method correctly returns a receive-only channel, preventing external code from closing or sending on it. The isStopFileEvent helper properly filters for Create events on the specific stop file name.


245-250: LGTM! Proper stop file handling in the event loop.

The detection, logging, channel close, and immediate return sequence ensures clean shutdown semantics. Closing fileStopChan signals observers before returning, allowing the daemon to coordinate its shutdown.


296-306: No issue: Design is correct and avoids goroutine leaks.

The daemon properly handles shutdown without blocking on StopChan() when Close() is called. After the main goroutine exits the select at line 268–270 (from either signal or fileStopChan closure), it immediately calls Close() at line 278, which closes the internal stopChan. This causes the Run() loop to exit via line 236–238. The daemon then waits for loopDone at line 281, ensuring the worker goroutine completes before returning. No caller blocks on StopChan() during Close(), and the two-channel design (internal stopChan for Close(), external fileStopChan for .stop file detection) is intentional and correct.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +158 to +162
// Verify error message contains expected prefix
expectedPrefix := "Failed to send stop signal:"
if len(stderr.String()) < len(expectedPrefix) || stderr.String()[:len(expectedPrefix)] != expectedPrefix {
t.Errorf("Expected stderr to start with %q, got %q", expectedPrefix, stderr.String())
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Use strings.HasPrefix for clearer intent.

The manual string slicing works but is less readable than the idiomatic helper.

♻️ Suggested improvement
+import "strings"
+
 // Verify error message contains expected prefix
 expectedPrefix := "Failed to send stop signal:"
-if len(stderr.String()) < len(expectedPrefix) || stderr.String()[:len(expectedPrefix)] != expectedPrefix {
+if !strings.HasPrefix(stderr.String(), expectedPrefix) {
     t.Errorf("Expected stderr to start with %q, got %q", expectedPrefix, stderr.String())
 }
🤖 Prompt for AI Agents
In `@internal/cli/mailman_stop_test.go` around lines 158 - 162, Replace the manual
length check and slicing that compares stderr.String() to expectedPrefix with
the idiomatic strings.HasPrefix call: import the strings package, then use if
!strings.HasPrefix(stderr.String(), expectedPrefix) { t.Errorf(... ) }
referencing the existing stderr variable and expectedPrefix constant in the test
(mailman_stop_test.go) to make the intent clearer and avoid manual bounds
checks.

Comment on lines +29 to +31
if err != nil {
repoRoot, _ = os.Getwd()
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider logging or documenting the silent Getwd fallback behavior.

If both FindGitRoot and Getwd fail (rare but possible), the empty repoRoot will cause a predictable OpenFile error. The current behavior is acceptable since the error will surface at file creation, but documenting this edge case in the function comment could help future maintainers.

🤖 Prompt for AI Agents
In `@internal/cli/mailman_stop.go` around lines 29 - 31, The code silently falls
back to os.Getwd when FindGitRoot fails and leaves repoRoot empty if Getwd also
fails; update mailman_stop.go to either log that fallback and any Getwd error
(in the block where err != nil and repoRoot is assigned) or add a clear function
comment above the relevant function noting that repoRoot may be empty if both
FindGitRoot and os.Getwd fail, referencing FindGitRoot, os.Getwd, and repoRoot
so future maintainers understand the edge case.

Comment on lines +21 to +27
**Operations**:
| Operation | Actor | Trigger |
|-----------|-------|---------|
| Create | CLI (stop command) | User runs `agentmail mailman stop` |
| Detect | Daemon (file watcher) | fsnotify CREATE event |
| Delete | Daemon | During graceful shutdown |

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add blank lines around the table for markdown compliance.

Per markdownlint MD058, tables should be surrounded by blank lines for consistent rendering across markdown parsers.

📝 Suggested fix
 **Operations**:
+
 | Operation | Actor | Trigger |
 |-----------|-------|---------|
 | Create | CLI (stop command) | User runs `agentmail mailman stop` |
 | Detect | Daemon (file watcher) | fsnotify CREATE event |
 | Delete | Daemon | During graceful shutdown |
+
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
**Operations**:
| Operation | Actor | Trigger |
|-----------|-------|---------|
| Create | CLI (stop command) | User runs `agentmail mailman stop` |
| Detect | Daemon (file watcher) | fsnotify CREATE event |
| Delete | Daemon | During graceful shutdown |
**Operations**:
| Operation | Actor | Trigger |
|-----------|-------|---------|
| Create | CLI (stop command) | User runs `agentmail mailman stop` |
| Detect | Daemon (file watcher) | fsnotify CREATE event |
| Delete | Daemon | During graceful shutdown |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

22-22: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🤖 Prompt for AI Agents
In `@specs/012-mailman-stop/data-model.md` around lines 21 - 27, The MD058 lint
failure is caused by the table not being surrounded by blank lines; edit the
section containing the "**Operations**:" heading and the subsequent markdown
table so there is an empty line before the table (between the "**Operations**:"
line and the table) and an empty line after the table, ensuring the table is
separated by blank lines for compliant rendering.

Comment on lines +41 to +53
**Implementation**:
```go
// Atomic create - fails if file exists
f, err := os.OpenFile(stopFilePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600)
if err != nil {
if os.IsExist(err) {
return "Stop already pending"
}
return fmt.Sprintf("Failed to send stop signal: %v", err)
}
f.Close()
return "Stop signal sent"
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add blank lines around the fenced code block.

Markdownlint MD031 flags missing blank lines before/after fences. Add a blank line before and after the Go snippet for consistent rendering.

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

42-42: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
In `@specs/012-mailman-stop/research.md` around lines 41 - 53, The fenced Go code
block around the atomic create snippet needs surrounding blank lines to satisfy
markdownlint MD031: insert one blank line immediately before the ```go fence
that starts the block and one blank line immediately after the closing ``` fence
so the snippet with os.OpenFile(stopFilePath, os.O_CREATE|os.O_EXCL|os.O_WRONLY,
0600) and its defer/Close/return lines is separated from surrounding text.

Comment on lines +89 to +105
**Existing Code Reference** (`internal/daemon/daemon.go:249-271`):
```go
// Wait for shutdown signal or test stop
<-sigChan

// Close file watcher to stop the notification loop
if fileWatcher != nil {
_ = fileWatcher.Close()
}

<-loopDone // Wait for loop to finish

// Clean up PID file on shutdown
_ = DeletePID(repoRoot)

return 0
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add blank lines around the fenced code block.

Markdownlint MD031 flags missing blank lines before/after fences. Add a blank line before and after this code block.

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

90-90: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
In `@specs/012-mailman-stop/research.md` around lines 89 - 105, Add a blank line
immediately before and immediately after the fenced code block that shows the
"Existing Code Reference (`internal/daemon/daemon.go:249-271`)" in
specs/012-mailman-stop/research.md so it complies with markdownlint MD031;
locate the triple-backtick fence that wraps the Go snippet (the block containing
"<-sigChan" through "return 0") and insert one empty line above the opening ```
and one empty line below the closing ``` to fix the lint error.

Comment on lines +106 to +111
```
Phase 1 (Setup) → No dependencies
Phase 2 (US1) → Depends on Phase 1
Phase 3 (US2) → Depends on Phase 2 (shares MailmanStop function)
Phase 4 (Polish) → Depends on all user stories
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Specify a language for the fenced code block.

Markdownlint MD040 requires a language tag. Use ```text (or a more specific language if applicable).

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

106-106: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In `@specs/012-mailman-stop/tasks.md` around lines 106 - 111, The fenced code
block showing the phase dependency list lacks a language tag and triggers
markdownlint MD040; update the block opener from ``` to include a language
(e.g., use ```text or a more specific language) so the block containing "Phase 1
(Setup) → No dependencies" through "Phase 4 (Polish) → Depends on all user
stories" has a proper language identifier.

@UserAd UserAd merged commit 7e26655 into main Jan 20, 2026
3 checks passed
@UserAd UserAd deleted the 012-mailman-stop branch January 20, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant